Fix invalid path construction in createDirectoryRecursively on Windows#93
Open
Fix invalid path construction in createDirectoryRecursively on Windows#93
Conversation
Runnin4ik
changed the title
Member
|
Hi @Runnin4ik, thank you for the analysis and the fix! Could you add something to the tests? |
- Adds a test case using sys_get_temp_dir() to verify that createDirectoryRecursively handles absolute paths correctly on Windows (e.g. 'C:\...') and Linux. This ensures the fix for the invalid path construction is verified and prevents future regressions.
Author
Hi @kelunik, I've added the requested test case covering absolute paths. |
kelunik
reviewed
Jan 21, 2026
| } | ||
|
|
||
| $parent = \dirname($dir); | ||
| if ($parent !== $baseDir && \str_starts_with($parent, $baseDir) && $driver->getStatus($parent)) { |
Member
There was a problem hiding this comment.
Why do we need the checks here?
Author
There was a problem hiding this comment.
I added them as a safeguard to ensure we don't accidentally delete the system temp directory, but I agree they are redundant here. I will simplify the code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug in
UvFilesystemDriver::createDirectoryRecursivelywhere creating directories with absolute paths on Windows failed withuverrorCould not create directory.The Issue
The current implementation unconditionally prepends
DIRECTORY_SEPARATORto every path chunk derived fromexplode().On Windows:
An absolute path like
E:\Projectexplodes into['E:', 'Project'].The loop immediately prepends a separator to the first element:
$tmpPathbecomes\E:(instead of justE:).While PHP's
mkdirmight handle this, the low-leveluv_fs_mkdirfunction treats\E:as an invalid path on Windows, causing the operation to fail immediately.On Linux:
An absolute path like
/var/wwwexplodes into['', 'var', 'www'].The loop turns the first empty element into
/.Then it appends the next element with a separator:
//var.Since Linux treats double slashes (
//) as a single slash, the bug was masked and the code worked by accident.The Fix
I modified the loop logic to correctly handle the separator appending:
E:staysE:(no leading slash)./varstays/var(leading slash preserved, no double slashes).Verification
Tested locally on Windows 11 with PHP 8.5.1 and
ext-uv. Directory creation now works correctly with absolute paths.